Don't fail a URLSessionTask on a 401 response #2255
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The basic authentication implementation fails a URLSessionTask with an error if a status code of 401 is received but we aren't able to build a URLProtectionSpace, possibly due to the absence of a the WWW-Authenticate response header. A valid HTTP response object is received in this case. It may not possible to go ahead with the `urlSession(_,task: task, didReceive:)` callback, either because a delegate isn't being used or because the `WWW-Authenticate` response header is absent. In such cases, the completion handler is invoked with the received response, NOT with an error. Note that basic authentication (swiftlang@4c1e8c4271c4e21000528 d3910f20a958d03f308#diff-14c6cb724eead661596e4468ae5ca638) wasn't available with Swift 4.x. It was first made available in development builds for Swift 5. Initially, when we failed to build a URLProtectionSpace, we failed with an error, which resulted in removing the task from the TaskRegistry. Further we went ahead and invoked the completion handler, which again tried to remove the same task from the TaskRegistry, resulting in a fatal error reported against Swift 5.0. As a fix to this, we decided to simply return after failing with an error (see swiftlang@69abd6dd396e1985a882a9 a39079801f84658fb3#diff-14c6cb724eead661596e4468ae5ca638) which means we skipped invoking the completion handler. It seems that the right thing to do is NOT fail with an error, but simply invoke the completion handler with the 401 response received.
|
@shahmishal Merging in swift-5.0-branch for monthlies seems to be disabled, is that intentional? |
|
@swift-ci test |
|
I was hoping to test this out with a snapshot but it looks like there aren't any for the Would it be possible to trigger a snapshot for the swift-5.0-branch? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry-picked from master, for inclusion in 5.0.2
Original MR description:
--
The basic authentication implementation fails a URLSessionTask with an error if
a status code of 401 is received but we aren't able to build a
URLProtectionSpace, possibly due to the absence of a the WWW-Authenticate
response header. A valid HTTP response object is received in this case. It may
not possible to go ahead with the
urlSession(_,task: task, didReceive:)callback, either because a delegate isn't being used or because the
WWW-Authenticateresponse header is absent. In such cases, the completionhandler is invoked with the received response, NOT with an error.
Note that basic authentication
(4c1e8c4271c4e21000528
d3910f20a958d03f308#diff-14c6cb724eead661596e4468ae5ca638) wasn't available
with Swift 4.x. It was first made available in development builds for Swift 5.
Initially, when we failed to build a URLProtectionSpace, we failed with an
error, which resulted in removing the task from the TaskRegistry. Further we
went ahead and invoked the completion handler, which again tried to remove the
same task from the TaskRegistry, resulting in a fatal error reported against
Swift 5.0. As a fix to this, we decided to simply return after failing with an
error (see
69abd6dd396e1985a882a9
a39079801f84658fb3#diff-14c6cb724eead661596e4468ae5ca638) which means we
skipped invoking the completion handler.
It seems that the right thing to do is NOT fail with an error, but simply
invoke the completion handler with the 401 response received.